I think this is probably unnecessary. This method is used in a single place for error reporting on RestoreSnapshotRequest which is user facing. We don't expect it to be set on such request and hence no need to check here. I think it's might be the reason why we don't have everything here.

PS: It might be helpful to make each option explicitly declare whether it's internal or not so that their usage is easier to reason with. Not suggesting it for this PR. Just writing it down as an idea.

We could - it exists to prevent xcontent parsing from erroring here: https://github.com/elastic/elasticsearch/blob/main/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/restore/RestoreSnapshotRequest.java#L582

We'd change it so that we don't write any new xcontent if the field is false, then we check if the field is present before parsing it here: https://github.com/elastic/elasticsearch/blob/main/server/src/main/java/org/elasticsearch/action/support/IndicesOptions.java#L1136

Does that sound good? I can push the change as a separate commit so we can see what it looks like and revert it if we don't like it

We probably don't want to expose this new option to REST layer at all since it is internally only. For example, the parsing and toXContent methods of GatekeeperOptions do not involve any of the allowXxx options since they are internal, i.e. not configurable by end-users as described in its javadoc. Along this line of reasoning, I think we can just have no-op parsring and toXContent methods here to avoid inadverntly exposing it to end-users. It means RestoreSnapshotRequest will throw an exception if user specifies the new option and that is desirable. Overall I think it is easier to expose it later if necessary while more difficult to hide if already exposed.

PS: There is some fuzziness on how it should work for background tasks such as ML datafeed. I think the previous discussion seems to prefer inject the indicesOption when necessary, similar to how it is injected at the REST layer for ResolveIndexRequest. In another word, we don't plan to rely on parsing it from the job's configs.

I think we do need a way to capture it in the job config though, because logs will resolve differently over time. We would likely want it such that each time the ML datafeed (or Transform) runs, it resolves logs based on the current state of the system. The alternative is resolving once when the job is created, logs -> p1:logs, -p2:logs and then a future p3:logs would be omitted from the job.

I assume it's easier to add toXContent methods later than it is to remove toXContent methods, so we can start with them being no-op and go from there?

I think we do need a way to capture it in the job config though, because logs will resolve differently over time.

This is the fuziness part for me. An alternative to capturing is to inject it when the job runs similar to how we plan to inject it for RestResolveIndexAction. I am not sure exactly how that would look like yet but feels theorectically possible.

I assume it's easier to add toXContent methods later than it is to remove toXContent methods

Yep exactly my thought as well.

Could we add an entry to the java docs above here for this?